Fix AST parsing issues with function scope, generics, and test packages#12
Closed
fank wants to merge 1 commit intosamlitowitz:masterfrom
Closed
Fix AST parsing issues with function scope, generics, and test packages#12fank wants to merge 1 commit intosamlitowitz:masterfrom
fank wants to merge 1 commit intosamlitowitz:masterfrom
Conversation
This commit addresses several parsing issues that caused errors when analyzing large codebases: 1. Function-scoped declarations: The visitor now stops descending into function bodies after emitting FuncDecl nodes. This prevents function-local variables from being incorrectly tracked as file-level declarations, which was causing "duplicate declaration" errors. 2. File emission order: Fixed the AST visitor to properly track package context when emitting File nodes. Previously, all files were emitted at once when processing a Package node, causing ImportSpec nodes to be assigned to the wrong file. Now File nodes are emitted as they're encountered during the walk, using the stored package context to look up filenames. 3. Blank identifier: Added logic to skip tracking "_" declarations since the blank identifier can legitimately appear multiple times in a file (e.g., for compile-time interface checks). 4. External test packages: Added filtering to skip packages with names ending in "_test" since these are external test packages that cannot be imported and won't cause import cycles. 5. Generic type receivers: Added support for extracting receiver names from generic types like Foo[T] and *Foo[T]. The new extractTypeName function handles ast.IndexExpr and ast.IndexListExpr for generic types with one or multiple type parameters. 6. Error messages: Enhanced error messages to include file paths for easier debugging when duplicate declarations are detected. These fixes allow the tool to successfully analyze complex codebases with generics, extensive test suites, and varied declaration patterns.
fank
force-pushed
the
fix-ast-parsing-issues
branch
from
6a999bd to
272033e
Compare
Owner
Resolved by #14 |
Owner
|
Some of these issues have been addressed and some are not issues. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes several parsing issues that were causing errors when analyzing large codebases:
Issues Fixed
1. Function-scoped declarations
Problem: Function-local variables were being tracked as file-level declarations, causing "duplicate declaration" errors when the same variable name was used in multiple functions.
Solution: Modified the visitor to return
nilafter emittingFuncDeclnodes, preventing descent into function bodies. Import cycle detection only needs package-level declarations.2. File emission order
Problem: All files were emitted when processing a
Packagenode, butImportSpecnodes were processed later during the AST walk. This caused all imports to be assigned to the last file in the package, leading to "duplicate import" errors.Solution: Added package context tracking (
currentPackage,currentDirName) to the visitor. File nodes are now emitted individually as they're encountered, with proper filename lookup from the package context.3. Blank identifier
Problem: The blank identifier
_was being tracked like a regular declaration, but it's used for compile-time checks and can appear multiple times.Solution: Skip tracking declarations named
_in theaddGenDeclfunction.4. External test packages
Problem: Packages ending in
_test(external test packages) were being processed, but they can coexist with regular packages in the same directory and cannot be imported.Solution: Added filtering in
main.goto skip packages with names ending in_test.5. Generic type receivers
Problem: Methods on generic types like
*Foo[T]orFoo[T, U]were causing errors because the receiver name extraction didn't handle generic type syntax.Solution: Added
extractTypeNamefunction that recursively extracts base type names fromast.IndexExprandast.IndexListExprnodes. Updated receiver type handling to support generic types.6. Error messages
Enhancement: Added file paths to all error messages for easier debugging.
Testing
These fixes were tested on a large codebase with:
The tool now successfully generates import cycle graphs for such codebases.